Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make SignalHandling supports different printer #238

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

z0ow
Copy link

@z0ow z0ow commented Aug 19, 2021

The backward::SignalHandling only supports outputting traces via backward::Printer takes a disadvantage to designers to customize their output style. This PR makes a derived class of backward::Printer can override print functions and adds class backward::SignalHandlingBase to allow designers to set their inherited printer as default.

The std::shared_ptr is brought into backward::SignalHandling to support polymorphism. This feature could make backward better if this change does not affect performance and compatibility. 🙏

@nmaludy
Copy link

nmaludy commented Dec 7, 2021

+1 would like to see this merged, i had a similar problem trying to customize the formatting of the SignalHandler output

@Trafo
Copy link

Trafo commented Nov 18, 2022

me too

@bombela
Copy link
Owner

bombela commented Nov 18, 2022

In this current state this would break backward compatibility by requiring C++11. So at least a new major version release would be required. But it is surely possible to maintain compatibility.

The printer and signal handlers were meant to be examples to use as a starting point for your own customization. You can write your own, specific to your project(s) and use what you want from backward-cpp.

With insight I should have had those examples in a different file. I wanted backward-cpp to be a single file to copy around for easy installation.

With that said, many people have asked for this feature over time. Maybe we can have our cake and eat it too.

@giacomocaironi
Copy link

Any updates?
I was able to create a custom SignalHandling class, but to achieve this I had to copy more than 300 lines of code from the example. If only we could inherit from the base class and override the functions this could be achieved in only a few lines of code.

@bombela
Copy link
Owner

bombela commented Jun 4, 2023

Why can't you inherit and override the base class?

@giacomocaironi
Copy link

I tried inheriting from the base class and overriding only handleSignal (I'm on linux), and it didn't change anything. I had to copy also the constructor which in turn needed some private methods. I think this is because the function in not marked as virtual in the base class, but I am not very familiar with c++ so I could be completely wrong.

Anyway my objection remains, if the PR could be merged it would simplify customizing the printer a lot. I understand that SignalHandling was only meant as an example, but it implements a lot of things, reusing it would be nice

@bombela
Copy link
Owner

bombela commented Jun 5, 2023

Dah; of course; the vtable is missing! In C++ the vtable pointer is within the object and I somehow forgot about that (I blame it on enjoying Rust too much ;)). I now remember that the destructor must be marked virtual too.

Instead of SignalHandlingBase::set_printer<Printer>() what about SignalHandling<P>(P&& printer)? That is, the signalhandling is given a Printer object to consume. Instead of setting a pointer to a printer. It somehow strikes me as being harder to misuse. It means you cannot modify the Printer after the fact. But maybe this is a good thing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants